Handling of MET by event, updated Jet Energy Corrections handling and b-tagging with jet tagging efficiencie - #115
Open
kerstinlovisa wants to merge 14 commits into
Open
Handling of MET by event, updated Jet Energy Corrections handling and b-tagging with jet tagging efficiencie#115kerstinlovisa wants to merge 14 commits into
kerstinlovisa wants to merge 14 commits into
Conversation
added 5 commits
February 23, 2024 10:16
…on sizes. Removing hardcoded events branch name.
Collaborator
Author
|
I thought I could do two separate idenpendent PRs at the same time but this one relies on the file in the other PR, so once that one is merged I can make sure this one passes |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors MET handling to be branch-name driven at the Event/NanoEvent level (supporting MET vs PuppiMET and chained updates), and updates jet correction/smearing + b-tagging SF logic to use JES/JER-updated jet kinematics and tagging efficiencies.
Changes:
- Introduces
Event::UpdateMetVariables()+GetMetPt/Phi()with configurablemetBranchName, and updates processors to propagate MET through JES/JER/XY corrections. - Adds JES/JER jet variables (
pt_JES,pt_smeared, etc.) and updates jet four-vector/SF computations to use smeared values. - Extends scale factor handling to support jet tagging efficiencies, expanded JEC configuration, and optional statistical variations for muon SFs.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/extensions/src/NanoJet.cpp | Adds JES/JER mass/pt accessors, updated b-tagging SF logic with efficiencies, and new JEC/JER interfaces. |
| libs/extensions/src/NanoEventProcessor.cpp | Switches MET usage to event-driven getters and adds Puppi MET JES, MET XY, and unclustered energy handling. |
| libs/extensions/src/NanoEvent.cpp | Updates MET 4-vector construction to use updated MET getters; uses NanoJet pt in vetoes. |
| libs/extensions/include/NanoJet.hpp | Exposes new JES/JER accessors and updated SF/JEC method signatures. |
| libs/extensions/include/NanoEventProcessor.hpp | Updates b-tagging SF API and declares new MET/JES/XY correction helpers. |
| libs/extensions/include/NanoEvent.hpp | Adds MET branch/getter forwarding to Event and exposes IsData(). |
| libs/core/src/ScaleFactorsManager.cpp | Updates JEC loading strategy, adds jet tagging efficiency lookup, and extends muon SF outputs. |
| libs/core/src/Event.cpp | Adds configurable MET branch name and a mechanism to update/read “current” MET variables. |
| libs/core/src/ConfigManager.cpp | Generalizes GetExtraEventCollections to accept an alternate config key name. |
| libs/core/include/ScaleFactorsManager.hpp | Updates CorrectionArgType and adds new scale factor APIs. |
| libs/core/include/PhysicsObject.hpp | Adds HasBranch() helper used by NanoJet JES/JER accessors. |
| libs/core/include/Event.hpp | Declares MET branch/update/getter APIs on Event. |
| libs/core/include/ConfigManager.hpp | Updates GetExtraEventCollections signature with a default config key. |
| configs/examples/scale_factors_config.py | Expands/modernizes JEC/JER config and adds jet efficiency maps + muon “stat” variations. |
Suppressed comments (2)
libs/extensions/src/NanoJet.cpp:190
- AddSmearedPtByResolution() now smears and stores pt_smeared based on pt_JES (pt variable), but the JER scale factor / resolution is computed using GetPt() (raw pt). This inconsistency can bias the smearing and gen-jet matching thresholds.
This issue also appears on line 195 of the same file.
// ScaleFactor
map<string, float> jerSF =
scaleFactorsManager.GetJetEnergyResolutionScaleFactorAndPtResolution((float)physicsObject->Get("eta"), GetPt(), rho);
libs/extensions/src/NanoJet.cpp:203
- The JER smearing factor inputs use {"JetPt", GetPt()} but the function applies the factor to pt_JES (pt). JetPt should match the pt being smeared to avoid inconsistent factors.
map<string, CorrectionArgType> inputs = {
{"JetPt", GetPt()},
{"JetEta", (float)physicsObject->Get("eta")},
{"GenPt", (double)genPt},
{"Rho", (double)rho},
{"EventID", (double)eventID},
{"JER", (double)jerSF["PtResolution"]},
{"JERSF", (double)jerSF["systematic"]},
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| info() << "------------------------------------\n" << endl; | ||
| } | ||
| if (ShouldApplyScaleFactor("pileup")) ReadPileupSFs(); | ||
| // if (ShouldApplyScaleFactor("pileup")) ReadPileupSFs(); |
Collaborator
Author
There was a problem hiding this comment.
This one I actually don't know what to do about, if uncommented it complains that the "pileupScaleFactorsPath" and "pileupScaleFactorsHistName" don't exist - becasue we don't use the custom PU SFs
Owner
There was a problem hiding this comment.
Should we maybe remove this custom pileup mechanism altogether?
| using CorrectionRef = DummyCorrectionRef; | ||
| using CompoundCorrectionRef = DummyCorrectionRef; | ||
| using CorrectionArgType = std::variant<long, double, std::string>; | ||
| using CorrectionArgType = std::variant<int, double, std::string>; |
Collaborator
Author
There was a problem hiding this comment.
I had to change this to fix the compile errors - because the default from correctionlib is int so I needed to match it
Owner
There was a problem hiding this comment.
Ah, this is the long-standing annoying issue... We tried several things to fix it. I think the true solution is: can you please update your correctionlib to the latest version?
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note that MET is now handled by Event by a met branch name so that it knows if it should be MET or PuppiMET variables, and to handle any update to MET after corrections/smearing